From 8960dd185029ab0ba319f5b3ed8d2aa22ebeb6f1 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 31 Oct 2014 18:39:39 -0700 Subject: [PATCH] Implement build-dependencies This adds a flavor of Dependency for build dependencies. Build dependencies are only ever used when building build commands themselves. --- src/cargo/core/dependency.rs | 25 ++++++-- src/cargo/ops/cargo_rustc/context.rs | 23 +++---- src/cargo/ops/cargo_rustc/custom_build.rs | 7 ++- src/cargo/ops/cargo_rustc/job_queue.rs | 9 ++- src/cargo/ops/cargo_rustc/mod.rs | 42 ++++++------- src/cargo/util/toml.rs | 27 +++++--- tests/resolve.rs | 7 ++- tests/test_cargo_compile_custom_build.rs | 75 +++++++++++++++++++++++ 8 files changed, 159 insertions(+), 56 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index fe83cf716..eefe72102 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -10,7 +10,7 @@ pub struct Dependency { source_id: SourceId, req: VersionReq, specified_req: Option, - transitive: bool, + kind: Kind, only_match_name: bool, optional: bool, @@ -22,6 +22,13 @@ pub struct Dependency { only_for_platform: Option, } +#[deriving(PartialEq, Clone, Show)] +pub enum Kind { + Normal, + Development, + Build, +} + impl Dependency { /// Attempt to create a `Dependency` from an entry in the manifest. /// @@ -55,7 +62,7 @@ impl Dependency { name: name.to_string(), source_id: source_id.clone(), req: VersionReq::any(), - transitive: true, + kind: Normal, only_match_name: true, optional: false, features: Vec::new(), @@ -83,8 +90,8 @@ impl Dependency { &self.source_id } - pub fn transitive(mut self, transitive: bool) -> Dependency { - self.transitive = transitive; + pub fn kind(mut self, kind: Kind) -> Dependency { + self.kind = kind; self } @@ -132,7 +139,15 @@ impl Dependency { } /// Returns false if the dependency is only used to build the local package. - pub fn is_transitive(&self) -> bool { self.transitive } + pub fn is_transitive(&self) -> bool { + match self.kind { + Normal | Build => true, + Development => false, + } + } + pub fn is_build(&self) -> bool { + match self.kind { Build => true, _ => false } + } pub fn is_optional(&self) -> bool { self.optional } /// Returns true if the default features of the dependency are requested. pub fn uses_default_features(&self) -> bool { self.default_features } diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 146cdb36a..f6ca433fa 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -155,7 +155,7 @@ impl<'a, 'b: 'a> Context<'a, 'b> { Vacant(entry) => { entry.set(req); } }; - for &(pkg, dep) in self.dep_targets(pkg).iter() { + for &(pkg, dep) in self.dep_targets(pkg, target).iter() { self.build_requirements(pkg, dep, req, visiting); } @@ -229,25 +229,26 @@ impl<'a, 'b: 'a> Context<'a, 'b> { /// For a package, return all targets which are registered as dependencies /// for that package. - pub fn dep_targets(&self, pkg: &Package) -> Vec<(&'a Package, &'a Target)> { + pub fn dep_targets(&self, pkg: &Package, target: &Target) + -> Vec<(&'a Package, &'a Target)> { let deps = match self.resolve.deps(pkg.get_package_id()) { None => return vec!(), Some(deps) => deps, }; - deps.map(|pkg_id| self.get_package(pkg_id)).filter_map(|pkg| { + deps.map(|id| self.get_package(id)).filter(|dep| { + // If this target is a build command, then we only want build + // dependencies, otherwise we want everything *other than* build + // dependencies. + let pkg_dep = pkg.get_dependencies().iter().find(|d| { + d.get_name() == dep.get_name() + }).unwrap(); + target.get_profile().is_custom_build() == pkg_dep.is_build() + }).filter_map(|pkg| { pkg.get_targets().iter().find(|&t| self.is_relevant_target(t)) .map(|t| (pkg, t)) }).collect() } - /// For a package, return all targets which are registered as build - /// dependencies for that package. - pub fn build_dep_targets(&self, _pkg: &Package) - -> Vec<(&'a Package, &'a Target)> { - // FIXME: needs implementation - vec![] - } - /// Gets a package for the given package id. pub fn get_package(&self, id: &PackageId) -> &'a Package { self.package_set.iter() diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 5f161dc19..b46505f12 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -40,7 +40,7 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) // Start preparing the process to execute, starting out with some // environment variables. let profile = target.get_profile(); - let mut p = super::process(to_exec, pkg, cx) + let mut p = super::process(to_exec, pkg, target, cx) .env("OUT_DIR", Some(&build_output)) .env("CARGO_MANIFEST_DIR", Some(pkg.get_manifest_path() .dir_path() @@ -70,7 +70,10 @@ pub fn prepare(pkg: &Package, target: &Target, cx: &mut Context) // This information will be used at build-time later on to figure out which // sorts of variables need to be discovered at that time. let lib_deps = { - cx.dep_targets(pkg).iter().filter_map(|&(pkg, _)| { + let non_build_target = pkg.get_targets().iter().find(|t| { + !t.get_profile().is_custom_build() + }).unwrap(); + cx.dep_targets(pkg, non_build_target).iter().filter_map(|&(pkg, _)| { pkg.get_manifest().get_links() }).map(|s| s.to_string()).collect::>() }; diff --git a/src/cargo/ops/cargo_rustc/job_queue.rs b/src/cargo/ops/cargo_rustc/job_queue.rs index e11bf0da7..86d8b9982 100644 --- a/src/cargo/ops/cargo_rustc/job_queue.rs +++ b/src/cargo/ops/cargo_rustc/job_queue.rs @@ -250,9 +250,14 @@ impl<'a> Dependency<(&'a Resolve, &'a PackageSet)> match stage { StageStart => Vec::new(), + // Building the build command itself starts off pretty easily,we + // just need to depend on all of the library stages of our own build + // dependencies (making them available to us). StageBuildCustomBuild => { - // FIXME: build dependencies should come into play here - vec![(id, StageStart)] + let mut base = vec![(id, StageStart)]; + base.extend(deps.filter(|&(_, dep)| dep.is_build()) + .map(|(id, _)| (id, StageLibraries))); + base } // When running a custom build command, we need to be sure that our diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index f6aae0cad..dc9798e5c 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -264,6 +264,9 @@ fn compile_custom_old(pkg: &Package, cmd: &str, Some(profile) => profile, None => return Err(internal(format!("no profile for {}", cx.env()))) }; + // Just need a target which isn't a custom build command + let target = &pkg.get_targets()[0]; + assert!(!target.get_profile().is_custom_build()); // TODO: this needs to be smarter about splitting let mut cmd = cmd.split(' '); @@ -272,7 +275,7 @@ fn compile_custom_old(pkg: &Package, cmd: &str, let layout = cx.layout(pkg, KindTarget); let output = layout.native(pkg); let old_output = layout.proxy().old_native(pkg); - let mut p = try!(process(cmd.next().unwrap(), pkg, cx)) + let mut p = try!(process(cmd.next().unwrap(), pkg, target, cx)) .env("OUT_DIR", Some(&output)) .env("DEPS_DIR", Some(&output)) .env("TARGET", Some(cx.target_triple())) @@ -294,7 +297,7 @@ fn compile_custom_old(pkg: &Package, cmd: &str, } - for &(pkg, _) in cx.dep_targets(pkg).iter() { + for &(pkg, _) in cx.dep_targets(pkg, target).iter() { let name: String = pkg.get_name().chars().map(|c| { match c { '-' => '_', @@ -389,7 +392,7 @@ fn rustc(package: &Package, target: &Target, fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>, cx: &Context, req: PlatformRequirement) -> CargoResult> { - let base = try!(process("rustc", package, cx)); + let base = try!(process("rustc", package, target, cx)); let base = build_base_args(cx, base, package, target, crate_types.as_slice()); let target_cmd = build_plugin_args(base.clone(), cx, package, target, KindTarget); @@ -415,7 +418,7 @@ fn rustdoc(package: &Package, target: &Target, let kind = KindTarget; let pkg_root = package.get_root(); let cx_root = cx.layout(package, kind).proxy().dest().join("doc"); - let rustdoc = try!(process("rustdoc", package, cx)).cwd(pkg_root.clone()); + let rustdoc = try!(process("rustdoc", package, target, cx)).cwd(pkg_root.clone()); let mut rustdoc = rustdoc.arg(target.get_src_path()) .arg("-o").arg(cx_root) .arg("--crate-name").arg(target.get_name()); @@ -584,7 +587,6 @@ fn build_deps_args(mut cmd: ProcessBuilder, target: &Target, package: &Package, // Traverse the entire dependency graph looking for -L paths to pass for // native dependencies. // OLD-BUILD: to-remove - // FIXME: traverse build deps for build cmds let mut dirs = Vec::new(); each_dep(package, cx, |pkg| { if pkg.get_manifest().get_build().len() > 0 { @@ -595,25 +597,17 @@ fn build_deps_args(mut cmd: ProcessBuilder, target: &Target, package: &Package, cmd = cmd.arg("-L").arg(dir); } - if target.get_profile().is_custom_build() { - // Custom build commands don't link to any other targets in the package, - // and they also link to all build dependencies, not normal dependencies - for &(pkg, target) in cx.build_dep_targets(package).iter() { - cmd = try!(link_to(cmd, pkg, target, cx, kind)); - } - } else { - for &(pkg, target) in cx.dep_targets(package).iter() { - cmd = try!(link_to(cmd, pkg, target, cx, kind)); - } + for &(pkg, target) in cx.dep_targets(package, target).iter() { + cmd = try!(link_to(cmd, pkg, target, cx, kind)); + } - let targets = package.get_targets().iter().filter(|target| { - target.is_lib() && target.get_profile().is_compile() - }); + let targets = package.get_targets().iter().filter(|target| { + target.is_lib() && target.get_profile().is_compile() + }); - if target.is_bin() { - for target in targets.filter(|f| !f.is_staticlib()) { - cmd = try!(link_to(cmd, package, target, cx, kind)); - } + if target.is_bin() && !target.get_profile().is_custom_build() { + for target in targets.filter(|f| !f.is_staticlib()) { + cmd = try!(link_to(cmd, package, target, cx, kind)); } } @@ -643,7 +637,7 @@ fn build_deps_args(mut cmd: ProcessBuilder, target: &Target, package: &Package, } } -pub fn process(cmd: T, pkg: &Package, +pub fn process(cmd: T, pkg: &Package, target: &Target, cx: &Context) -> CargoResult { // When invoking a tool, we need the *host* deps directory in the dynamic // library search path for plugins and such which have dynamic dependencies. @@ -655,7 +649,7 @@ pub fn process(cmd: T, pkg: &Package, // Also be sure to pick up any native build directories required by plugins // or their dependencies let mut native_search_paths = HashSet::new(); - for &(dep, target) in cx.dep_targets(pkg).iter() { + for &(dep, target) in cx.dep_targets(pkg, target).iter() { if !target.get_profile().is_for_host() { continue } each_dep(dep, cx, |dep| { if dep.get_manifest().get_build().len() > 0 { diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index a292d6a2e..f2abb2dd1 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -10,8 +10,9 @@ use semver; use serialize::{Decodable, Decoder}; use core::SourceId; -use core::manifest::{LibKind, Lib, Dylib, Profile, ManifestMetadata}; use core::{Summary, Manifest, Target, Dependency, PackageId}; +use core::dependency::{Build, Development}; +use core::manifest::{LibKind, Lib, Dylib, Profile, ManifestMetadata}; use core::package_id::Metadata; use util::{CargoResult, Require, human, ToUrl, ToSemver}; @@ -210,6 +211,7 @@ pub struct TomlManifest { bench: Option>, dependencies: Option>, dev_dependencies: Option>, + build_dependencies: Option>, features: Option>>, target: Option>, } @@ -481,13 +483,20 @@ impl TomlManifest { }; // Collect the deps - try!(process_dependencies(&mut cx, false, None, self.dependencies.as_ref())); - try!(process_dependencies(&mut cx, true, None, self.dev_dependencies.as_ref())); + try!(process_dependencies(&mut cx, self.dependencies.as_ref(), + |dep| dep)); + try!(process_dependencies(&mut cx, self.dev_dependencies.as_ref(), + |dep| dep.kind(Development))); + try!(process_dependencies(&mut cx, self.build_dependencies.as_ref(), + |dep| dep.kind(Build))); if let Some(targets) = self.target.as_ref() { for (name, platform) in targets.iter() { - try!(process_dependencies(&mut cx, false, Some(name.clone()), - platform.dependencies.as_ref())); + try!(process_dependencies(&mut cx, + platform.dependencies.as_ref(), + |dep| { + dep.only_for_platform(Some(name.clone())) + })); } } } @@ -528,8 +537,9 @@ impl TomlManifest { } } -fn process_dependencies<'a>(cx: &mut Context<'a>, dev: bool, platform: Option, - new_deps: Option<&HashMap>) +fn process_dependencies<'a>(cx: &mut Context<'a>, + new_deps: Option<&HashMap>, + f: |Dependency| -> Dependency) -> CargoResult<()> { let dependencies = match new_deps { Some(ref dependencies) => dependencies, @@ -568,8 +578,7 @@ fn process_dependencies<'a>(cx: &mut Context<'a>, dev: bool, platform: Option ["bar", dep("baz").transitive(false)]), - pkg!("baz" => ["bat", dep("bam").transitive(false)]), + pkg!("foo" => ["bar", dep("baz").kind(Development)]), + pkg!("baz" => ["bat", dep("bam").kind(Development)]), pkg!("bar"), pkg!("bat") )); let res = resolve(pkg_id("root"), - vec![dep("foo"), dep("baz").transitive(false)], + vec![dep("foo"), dep("baz").kind(Development)], &mut reg).unwrap(); assert_that(&res, contains(names(["root", "foo", "bar", "baz"]))); diff --git a/tests/test_cargo_compile_custom_build.rs b/tests/test_cargo_compile_custom_build.rs index d94861df2..9eac0c63f 100644 --- a/tests/test_cargo_compile_custom_build.rs +++ b/tests/test_cargo_compile_custom_build.rs @@ -540,3 +540,78 @@ test!(propagation_of_l_flags { {running} `rustc [..] --crate-name foo [..] -L bar[..]-L foo[..]` ", compiling = COMPILING, running = RUNNING).as_slice())); }) + +test!(build_deps_simple { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.5.0" + authors = [] + build = "build.rs" + [build-dependencies.a] + path = "a" + "#) + .file("src/lib.rs", "") + .file("build.rs", " + extern crate a; + fn main() {} + ") + .file("a/Cargo.toml", r#" + [project] + name = "a" + version = "0.5.0" + authors = [] + "#) + .file("a/src/lib.rs", ""); + + assert_that(p.cargo_process("build").arg("-v"), + execs().with_status(0) + .with_stdout(format!("\ +{compiling} a v0.5.0 (file://[..]) +{running} `rustc [..] --crate-name a [..]` +{compiling} foo v0.5.0 (file://[..]) +{running} `rustc build.rs [..] --extern a=[..]` +{running} `[..]foo-[..]build-script-build` +{running} `rustc [..] --crate-name foo [..]` +", compiling = COMPILING, running = RUNNING).as_slice())); +}) + +test!(build_deps_not_for_normal { + let (_, target) = ::cargo::ops::rustc_version().unwrap(); + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.5.0" + authors = [] + build = "build.rs" + [build-dependencies.a] + path = "a" + "#) + .file("src/lib.rs", "extern crate a;") + .file("build.rs", " + extern crate a; + fn main() {} + ") + .file("a/Cargo.toml", r#" + [project] + name = "a" + version = "0.5.0" + authors = [] + "#) + .file("a/src/lib.rs", ""); + + assert_that(p.cargo_process("build").arg("-v").arg("--target").arg(target), + execs().with_status(101) + .with_stderr("\ +[..]lib.rs[..] error: can't find crate for `a` +[..]lib.rs[..] extern crate a; +[..] ^~~~~~~~~~~~~~~ +error: aborting due to previous error +Could not compile `foo`. + +Caused by: + Process didn't exit successfully: [..] +")); +}) -- 2.30.2